TIKA-4835-spill-less - #3067
Closed
tballison wants to merge 4 commits into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets TIKA-4835 by reducing unnecessary temp-file spooling when parsing or detecting in-memory / stream-backed inputs, shifting several code paths to rely on TikaInputStream.enableRewind(...) + rewind() (governed by CacheMemoryBudget) instead of forcing getFile()/getPath().
Changes:
- Update PDF incremental-update xref scanning, ODF inline-picture handling, POIFS container detection, and image parsers (JPEG/TIFF/WebP) to prefer rewindable streams over spooling to temp files.
- Add focused “no temp file created” tests across PDF/ODF/POIFS/image modules.
- Improve digesting of translated embedded streams by buffering translated bytes in memory up to budget before spilling (
TranslatedBytes), and update release notes / one integration-test assertion accordingly.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tika-parsers/.../pdf/PDFParserNoTempFileTest.java | New test asserting PDF incremental-update xref scanning doesn’t spool in-memory input to disk. |
| tika-parsers/.../pdf/PDFParser.java | Avoids spooling for xref scan on in-memory inputs; adds rewind helpers to support multi-pass reads. |
| tika-parsers/.../odf/OpenDocumentParserNoTempFileTest.java | New test ensuring ODF parse doesn’t spool inline pictures for in-memory input. |
| tika-parsers/.../odf/OpenDocumentParser.java | Switch inline-picture detection from getFile()/reset() to enableRewind(...)/rewind(). |
| tika-parsers/.../microsoft/POIFSContainerDetectorNoTempFileTest.java | New test ensuring OLE2 detection keeps in-memory objects off disk and preserves stream position. |
| tika-parsers/.../microsoft/POIFSContainerDetector.java | Adds an in-memory POIFS path using getSeekableByteChannel() + budget/size limits, falling back to spooling when needed. |
| tika-parsers/.../image/ImageParsersNoTempFileTest.java | New test covering JPEG/TIFF/WebP parsers not spooling in-memory inputs. |
| tika-parsers/.../image/WebPParser.java | Replaces temp-file based multi-pass parsing with rewind between passes; uses stream-based overloads where possible. |
| tika-parsers/.../image/TiffParser.java | Uses rewind between XMP and metadata-extractor passes; adds stream-based parse path when not file-backed. |
| tika-parsers/.../image/JpegParser.java | Uses rewind between XMP and metadata-extractor passes; adds stream-based parse path when not file-backed. |
| tika-parsers/.../image/ImageXmp.java | Changes JPEG/WebP XMP extraction APIs from File to InputStream to avoid forced spooling. |
| tika-parsers/.../image/ImageMetadataExtractor.java | Adds InputStream overloads for JPEG/TIFF/WebP metadata extraction. |
| tika-parsers/.../rtf/RTFParserTest.java | Adjusts expected metadata-key count due to removal of temp-file filesystem tags for embedded images. |
| tika-core/src/test/.../TranslatedBytesTest.java | New unit tests for in-memory vs spill behavior and budget-based growth. |
| tika-core/src/main/.../TranslatedBytes.java | New output sink that buffers translated bytes in memory up to threshold/budget, then spills. |
| tika-core/src/main/.../DigestHelper.java | Uses TranslatedBytes instead of always writing translated streams to a temp file before digesting. |
| CHANGES.txt | Release-notes entry documenting spill reduction and the visible metadata change for embedded images. |
Suppressed comments (1)
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageXmp.java:131
- The standalone block (extra
{ ... }) in this method doesn’t add any scoping and looks like a leftover from a removed try-with-resources. Removing it would make the method easier to read and avoid confusing brace structure.
{
byte[] head = new byte[12];
if (IOUtils.read(in, head, 0, 12) < 12 || head[0] != 'R' || head[1] != 'I' ||
head[2] != 'F' || head[3] != 'F' || head[8] != 'W' || head[9] != 'E' ||
head[10] != 'B' || head[11] != 'P') {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+619
to
+627
| if (budget != null) { | ||
| if (budget.tryReserve(size) == 0) { | ||
| return null; | ||
| } | ||
| stream.addCloseableResource(() -> budget.release(size)); | ||
| } | ||
| POIFSFileSystem fs = new POIFSFileSystem(Channels.newInputStream(channel)); | ||
| stream.setOpenContainer(fs); | ||
| return getTopLevelNames(fs.getRoot()); |
Comment on lines
+324
to
+336
| // In-memory input is scanned from memory rather than spooled to a file for this | ||
| // pass. Rewind support is enabled first so the later parse (and a renderer's | ||
| // getPath(), if rendering is on) can re-read the content from the cache: the old | ||
| // getFile() spool used to provide that as a side effect. | ||
| boolean fileBacked = tikaInputStream.hasFile(); | ||
| try { | ||
| if (!fileBacked) { | ||
| tikaInputStream.enableRewind(parseContext.get(CacheMemoryBudget.class)); | ||
| } | ||
| try (RandomAccessRead ra = fileBacked ? | ||
| new RandomAccessReadBufferedFile(tikaInputStream.getFile()) : | ||
| new RandomAccessReadBuffer(CloseShieldInputStream.wrap(tikaInputStream))) { | ||
| StartXRefScanner xRefScanner = new StartXRefScanner(ra); |
Contributor
Author
|
Simplifying work here in a follow on PR. |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for your contribution to Apache Tika! Your help is appreciated!
Before opening the pull request, please verify that
TIKA-XXXX)[TIKA-XXXX] Issue or pull request title)./mvnw clean testmainbranch. If there are conflicts, please try to rebase the pull request branch on top of a freshly pulledmainbranchtika-bom/pom.xml.We will be able to faster integrate your pull request if these conditions are met. If you have any questions how to fix your problem or about using Tika in general, please sign up for the Tika mailing list. Thanks!